Skip to content

Fix market order price rounding for builder-deployed perp dexs - #311

Open
pucedoteth wants to merge 2 commits into
hyperliquid-dex:masterfrom
pucedoteth:fix-hip3-market-order-price-rounding
Open

Fix market order price rounding for builder-deployed perp dexs#311
pucedoteth wants to merge 2 commits into
hyperliquid-dex:masterfrom
pucedoteth:fix-hip3-market-order-price-rounding

Conversation

@pucedoteth

Copy link
Copy Markdown

Problem

Exchange._slippage_price decides how many decimal places a market-order price may have:

asset = self.info.coin_to_asset[coin]
# spot assets start at 10000
is_spot = asset >= 10_000
...
return round(float(f"{px:.5g}"), (6 if not is_spot else 8) - self.info.asset_to_sz_decimals[asset])

Spot asset ids are index + 10000, but builder-deployed (HIP-3) perp dex asset ids are 110000 + i * 10000 + index (see Info.__init__). Those are also >= 10_000, so every HIP-3 perp is classified as spot and rounded to 8 - szDecimals decimals instead of 6 - szDecimals.

market_open / market_close on a HIP-3 dex can therefore build a price with more decimal places than perps allow, and the exchange rejects the order. For an asset with szDecimals == 0 and a mid of 0.0012345678, a 5% buy slippage price comes out as 0.0012963 (7 decimals) where the perp limit is 6.

#225 fixed the all_mids lookup for HIP-3 dexs but left this check untouched.

Fix

Spot asset ids live in [10000, 110000), so bound the check on both sides:

is_spot = 10_000 <= asset < 110_000

Tests

Adds tests/exchange_test.py covering _slippage_price for a perp, a spot pair, and a builder-deployed perp asset. The last one fails on master (0.0012963 != 0.001296) and passes with the fix. The existing suite still passes (39 tests).

_slippage_price classifies an asset as spot with `asset >= 10_000`, but
builder-deployed (HIP-3) perp dex assets start at 110000, so they are
also caught by that check. Those perps are then rounded to 8 - szDecimals
decimals instead of 6 - szDecimals, and market_open/market_close can
produce a price with too many decimal places, which the exchange rejects.

Spot asset ids live in [10000, 110000), so bound the check on both sides.
@pucedoteth
pucedoteth force-pushed the fix-hip3-market-order-price-rounding branch from 46522b3 to 8f283b8 Compare August 17, 2026 00:11

@koriyoshi2041 koriyoshi2041 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified at 8f283b807d. The range matches Info's asset-ID layout: 109999 retains spot precision (0.0012963) while 110000 switches to perp precision (0.001296). The full test suite passes locally (39/39) on Python 3.11, and git diff --check is clean. The focused regression also fails on master and passes here.

One non-blocking test improvement would be to assert both adjacent boundary IDs in the regression, but the implementation is correct as written.

Per review: the regression asserted only that asset 110000 rounds as a
perp. Asset 109999 is the last spot id, so asserting it keeps spot
precision fixes the other end of the same boundary.

The two assertions fail independently, which is the point:

    is_spot = asset >= 10_000            (the original bug)
      -> 110000 assertion fails, 109999 still passes
    is_spot = 10_000 <= asset < 109_999  (upper bound off by one)
      -> 109999 assertion fails, 110000 still passes

39/39 pass.
@pucedoteth

Copy link
Copy Markdown
Author

Thanks for the review, and for reproducing it independently.

Took the test suggestion: the regression now asserts asset 109999 keeps spot precision (0.0012963) alongside 110000 switching to perp precision (0.001296), so the boundary is pinned from both sides.

They fail independently, which is what makes the pair worth having:

mutation 110000 assertion 109999 assertion
asset >= 10_000 (the original bug) fails passes
10_000 <= asset < 109_999 (upper bound off by one) passes fails

39/39 still pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants